Skip to content

Fix/terminal image rendering - #1084

Open
smarpitm wants to merge 2 commits into
CodebuffAI:mainfrom
smarpitm:fix/terminal-image-rendering
Open

Fix/terminal image rendering#1084
smarpitm wants to merge 2 commits into
CodebuffAI:mainfrom
smarpitm:fix/terminal-image-rendering

Conversation

@smarpitm

@smarpitm smarpitm commented Aug 22, 2026

Copy link
Copy Markdown

What

Fixes getKittyFormat() which invented format ids (101/102/103/104) that don't exist in the Kitty graphics protocol spec. The spec only defines f=24 (RGB), f=32 (RGBA), and f=100 (PNG).

Non-PNG payloads (JPEG, WebP, GIF) produced by the compression pipeline are now converted to PNG via Jimp before transmission, so the bytes always match f=100 . This prevents kitty/WezTerm/Ghostty terminals from silently dropping images.

Why

The original getKittyFormat() returned fabricated ids — f=102 for JPEG, f=103 for WebP, f=104 for GIF. Since image-handler compresses large images to JPEG, sending those bytes with f=102 to a real terminal causes the terminal to either error or silently drop the image (it has no such format). The tests only asserted the escape sequence contained f=102 , not that any real terminal accepts it.

Changes

  • cli/src/utils/terminal-images.ts — getKittyFormat() always returns 100 ; new convertToPngIfNeeded() converts non-PNG payloads to PNG via Jimp; generateKittyImageSequence() and renderInlineImage() are now async.
  • cli/src/components/blocks/image-block.tsx — useMemo → useEffect + useState to handle async renderInlineImage .
  • cli/src/components/image-card.tsx — await renderInlineImage(...) in the existing async loadThumbnail .

New tests

  • getKittyFormat block: returns 100 for all media types.
  • test.each (5 media types): validates f= in the escape sequence is always in {24, 32, 100} .
  • no fabricated format ids : multi-chunk JPEG payload — no f=101 / 102 / 103 / 104 in any chunk.
  • non-PNG converted to PNG : JPEG payload is re-encoded (original base64 absent from output).

Validation

  • bun run typecheck (cli) clean.
  • 25 new tests + 152 related tests pass.

Note

This PR builds on #1 (terminal detection + pwsh fallback). Merge that first.

DeepSeek V4 Flash (the default model) is multimodal: false and reads images via the server-side describe pipeline — for best results with photos, use MiniMax M3 / GPT-5.6 Luna / MiMo 2.5.

smarpitm and others added 2 commits August 11, 2026 23:33
@smarpitm
fix(cli): spec-compliant terminal image rendering + Windows clipboard…
a912ea6 
… fallback
@smarpitm @codebuff-team
fix(cli): spec-compliant kitty format ids + PNG conversion for non-PN…
b30b35b 
…G payloads

getKittyFormat() now always returns 100 (PNG) — the only format id that
all kitty-protocol terminals guarantee. Non-PNG payloads (JPEG, WebP, GIF)
are converted to PNG via Jimp before transmission, so the bytes always
match f=100. This prevents terminals from silently dropping images due to
fabricated format ids (101-104) that don't exist in the kitty spec.

renderInlineImage() is now async to support the Jimp conversion step.
image-block.tsx and image-card.tsx updated to handle async with
useEffect + state.

New tests validate that the escape sequence uses only spec-compliant
format ids (f=24|32|100) across all media types, and that no fabricated
ids (101-104) appear in any chunk.

🤖 Generated with Codebuff
Co-Authored-By: Codebuff <noreply@codebuff.com>
@github-actions

github-actions Bot commented Aug 22, 2026

Copy link
Copy Markdown
Contributor

PR checks failed

A couple of things need fixing before this can be reviewed.
None of them are about the code itself.

Title looks like a branch name.

Titles such as Fix/windows-conpty-leak come from the branch
rather than being written for a reader. Please rewrite it as a
sentence: Fix ANSI escape leak in Windows ConPTY.

Your title: Fix/terminal image rendering

Edit the PR and this check re-runs automatically.

@codebuff-team

codebuff-team commented Aug 22, 2026

Copy link
Copy Markdown
Contributor

The core diagnosis is right and worth porting: getKittyFormat() really did invent format ids (101–104) that don't exist in the kitty spec (only 24/32/100 are defined), and the multi-chunk logic in generateKittyImageSequence previously repeated a=T,f=...,c=...,r=... on every chunk instead of sending only m= on non-first chunks — both are real protocol bugs that would misrender or drop images on real terminals. The iTerm2 size= fix (byte length of decoded data, not base64 length) is also a genuine, well-reasoned correctness fix. Good test coverage for all three in terminal-images.test.ts.

But the PR is much bigger than its title suggests and mixes unrelated changes:

  • cli/src/utils/clipboard-image.ts — the new spawnPowerShell fallback-to-pwsh helper and timeout tweaks have nothing to do with terminal image rendering. Per the PR body itself, this depends on a separate unmerged PR ("Edit prompt #1 - terminal detection + pwsh fallback"), which is why it's showing up here — please rebase cleanly and drop it from this PR.
  • New terminal detection for WezTerm/Ghostty/Warp/Konsole in detectTerminalImageSupport is a feature addition, not part of "fixing" the existing kitty format bug. It may be reasonable but should be its own PR with its own justification (case-insensitive TERM_PROGRAM matching, env var additions in types/env.ts, etc.).
  • test/setup-scm-loader.ts is unrelated test-infra plumbing, presumably needed only because of the dependency on PR Edit prompt #1.
  • image-pipeline-integrity.test.ts is a large, mostly unrelated test file (compression/aspect-ratio/oversized-file tests) that doesn't touch the code being changed in this PR.

As submitted this reads like three or four PRs squashed together, several of which depend on an unmerged PR, which makes it hard for a maintainer to port cleanly by hand. Please split into: (1) the kitty format-id + chunking + iTerm2 size fix with its tests, and (2) the new terminal detection / clipboard fallback work as separate, independently reviewable PRs.

@codebuff-team codebuff-team added bot:triaged Classified by the community triage bot pr:needs-work Right idea, not mergeable as written labels Aug 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

bot:triaged Classified by the community triage bot pr:needs-work Right idea, not mergeable as written

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@smarpitm @codebuff-team